[opentitanlib] further refactors to UartConsole#28805
[opentitanlib] further refactors to UartConsole#28805nbdd0121 merged 6 commits intolowRISC:masterfrom
UartConsole#28805Conversation
Previously `set_break` needs to be on `ConsoleDevice` because `UartConsole` expects it to be there, so it can turn stdin breaks into console breaks. This feature is now moved to `opentitantool console` (which was the only place this feature was used), so the `set_break` doesn't need to be on `ConsoleDevice` anymore (and also this capability is only available on `Uart` impls anyway). Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
nbdd0121
force-pushed
the
uart-console
branch
from
2180a2d to
c5625b3
Compare
jwnrt
left a comment
jwnrt
left a comment
There was a problem hiding this comment.
Looks okay, thanks
Out of interest did you look for existing libraries to do the broadcast? Maybe something like the crossbeam mpmc?
| let mut inner = self.inner.lock().unwrap(); | ||
|
|
||
| let pos = inner.reader_pos[self.index]; | ||
| let index = if let Some(index) = inner.reader_pos.iter().position(|x| x.is_none()) { |
There was a problem hiding this comment.
Nit: This is slightly hard to follow as a one-liner, could you split the None index finder to a variable?
|
|
||
| // Runs an interactive console until CTRL_C is received. | ||
| pub fn interact<T>(&mut self, device: &T, stdout: Option<&mut dyn Write>) -> Result<ExitStatus> | ||
| pub fn interact<T>(&mut self, device: &T, quiet: bool) -> Result<ExitStatus> |
There was a problem hiding this comment.
Nit: I would personally prefer an API with an enum since it's not obvious what the Boolean does from the call site
#[derive(Clone, Copy, Debug)]
enum Echo {
Off,
On,
}
pub fn interact<T>(&mut self, device: &T, echo: Echo) -> Result<ExitStatus>There was a problem hiding this comment.
I plan to remove quiet (probably turn it into a field or setter method) given most users set it to false. Going to leave this to a future PR.
There's broadcast in tokio, but it is generic and so you can only do one-element-at-a-time, where I want to just handle bytes. |
This can allow multiple concurrent readers to all receive all data. Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
Reduce nesting level by 1. Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
We always use `stdout` if it's not `None`, so just use a boolean quiet flag instead. Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
nbdd0121
force-pushed
the
uart-console
branch
from
c5625b3 to
aceef00
Compare
2 participants
This adds an utility for UART (consoles)
Broadcasterwhich can be used to disseminate received data to multiple readers and each readers be able to read full history.This can be useful when there're multiple users of UART that need to handle inputs simultaneously. The
opentitantool's logfile option has been refactored to do this so it no longer needs to be part ofUartConsole.